Fix timing issue in Elasticsearch tests #481
Conversation
| @@ -39,8 +39,12 @@ | |||
|
|
|||
| @BeforeClass | |||
| public static void setup() throws IOException { | |||
| System.setProperty("es.set.netty.runtime.available.processors", "false"); | |||
There was a problem hiding this comment.
Could this obscure actual problems?
There was a problem hiding this comment.
Sorry, I'm still investigating and testing at my repository. Since this pull request is work in progress, I updated the subject.
For now, we (I and author of elasticsearch-cluster-runner library) think that actual problem is elasticsearch-cluster-runner can't run multiple cluster on same JVM at same time by internal structure of Elasticsearch. Maybe it can be resolved by merging Java test and Scala test.
takezoe
changed the title
because elasticsearch-cluster-runner doesn't support parallel execution formaly.
takezoe
force-pushed
the
fix-elasticsearch-test
branch
from
45d76e0 to
20839e9
Compare
Because elasticsearch tests are executed serially.
takezoe
force-pushed
the
fix-elasticsearch-test
branch
from
0195dbd to
cf9084f
Compare
takezoe
changed the title
|
@raboof This pull request disables parallel execution in elasticsearch project because elasticsearch-cluster-runner doesn't support parallel execution on same JVM formally. It might not be the best solution, but we couldn't find better one. |
build.sbt
Outdated
| @@ -93,6 +93,7 @@ lazy val elasticsearch = project | |||
| .enablePlugins(AutomateHeaderPlugin) | |||
| .settings( | |||
| name := "akka-stream-alpakka-elasticsearch", | |||
| parallelExecution in ThisBuild := false, | |||
There was a problem hiding this comment.
Maybe add a comment explaining why this was needed?
There was a problem hiding this comment.
Thanks, I fixed to disable only test execution and added a comment.
| runner.ensureYellow(); | ||
|
|
||
| //#init-client | ||
| client = RestClient.builder(new HttpHost("localhost", 9211)).build(); | ||
| client = RestClient.builder(new HttpHost("localhost", 9201)).build(); |
There was a problem hiding this comment.
When (intentionally) re-using the same port, you could consider putting the actual port number in a constant somewhere and referring to it from both places where you use it.
There was a problem hiding this comment.
I think sharing the port number is not necessary. These tests are independent and they don't have to use same port necessarily. It's just no longer necessary to use different ports.
There was a problem hiding this comment.
Agreed, using a constant make it more obvious that it's intentional though. But OK like this 👍
|
@raboof Fixed. |
|
Thanks! |
Disables parallel test because elasticsearch-cluster-runner which is used to run Elasticsearch for test doesn't support parallel execution on same JVM formally.
Fixes #479.